Skip to content

Conversation

@bmax
Copy link
Contributor

@bmax bmax commented Apr 25, 2020

Port from old library to new: logstash-plugins/logstash-input-jdbc#368

According to documentation (https://jdbc.postgresql.org/documentation/head/query.html) we need autocommit disabled. Work around is to create transaction and rollback always.

@bmax bmax force-pushed the fix_postgressql_fetching branch from 1798e97 to 39cfce3 Compare April 25, 2020 05:50
Copy link
Member

@robbavey robbavey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution @bmax - I have a couple of questions


it "should report the statements to logging" do
expect(plugin.logger).to receive(:debug).once
expect(plugin.logger).to receive(:debug).thrice
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we know why this needed to change?

paged_dataset.each do |row|
# Execute query in transaction cause PG driver require autocommit off for set fetch count
# See: https://jdbc.postgresql.org/documentation/head/query.html
db.transaction(rollback: :always) do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it might be possible to potentially use paged_each here?

If not, do we need to use transactions when paging is not enabled?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants